Added support for self-signed certificates#96
Added support for self-signed certificates#96kwilsonmg wants to merge 1 commit into1Password:mainfrom
Conversation
Support added for self-signed certificates. This addresses issue 1Password#95
| if is_async and certificate: | ||
| return AsyncClient(url, token, certificate) | ||
| elif is_async: | ||
| return AsyncClient(url, token) | ||
| return Client(url, token) | ||
| elif certificate: | ||
| return Client(url, token, certificate) | ||
| else: | ||
| return Client(url, token) |
There was a problem hiding this comment.
As the constructor params starts to grow, to avoid growing the amount of the params more in the future, I think it would be good to pass clientConfig: ClientConfig instead of passing a specific certificate param.
So, the idea is that ClientConfig class will have the certificate: str param, and in the future we can easily extend it with more, if we need them. In addition to that, we can make ClientConfig inherit the httpx.Client class, so we will be able to pass all the properties that httpx.Client accepts, and not only certificate.
Does it make sense?
There was a problem hiding this comment.
@volodymyrZotov That does make sense. I just did a search of this repo and don't see a "ClientConfig". Would that would be a new class created? If so, what sorts of parameters would you see it take? Certificate, url, token?
There was a problem hiding this comment.
Yes, that should be a new class. Ideally, having the same parameters as in httpx.Client and httpx.AsyncClient classes.
|
Created issue #99 to address this more broadly, so the client can be configured with different options. @kwilsonmg I'll appreciate your input there. I close this PR. |
2 participants
Support added for self-signed certificates. This addresses issue #95. I have tested and confirmed working with the synchronous client.